Skip to content

fix(review): address non-blocking review nits across the credibility-gate-farming series - #3615

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
claude/review-nits-cleanup
Jul 5, 2026
Merged

fix(review): address non-blocking review nits across the credibility-gate-farming series#3615
loopover-orb[bot] merged 1 commit into
mainfrom
claude/review-nits-cleanup

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Closes #3613

Summary

Cleans up 6 non-blocking "nits" the gittensory-orb review flagged on #3513 and
#3575 (both already merged) — no functional bugs, just polish left over from
that series:

  • src/review/unlinked-issue-match.ts:40: replaced the Unicode ellipsis in
    the truncated-diff marker with an ASCII ... (diff truncated), matching
    this repo's ASCII-source convention.
  • src/signals/unlinked-issue-candidates.ts: removed a duplicate "would"
    entry in STOPWORDS.
  • src/signals/unlinked-issue-candidates.ts: basename path-mention matching
    (issueMentionsChangedPath) now extracts path-like tokens from the issue
    body and requires an EXACT token match (or a longer path token ending in
    /basename), instead of a raw .includes() substring check — a basename
    like reader.ts no longer false-positives merely because it's a substring
    of an unrelated, longer filename such as csv-reader.ts. The full-path
    check is untouched (a repo-relative path is already distinctive enough that
    a substring match there isn't a realistic false-positive risk).
  • src/review/linked-issue-hard-rules.ts: resolveLinkedIssueHasOpenReference
    now bails out (fails open, returning true) once linkedIssues.length
    exceeds MAX_LINKED_ISSUE_NUMBERS, instead of firing an unbounded
    Promise.all fan-out of live GitHub fetches — reusing the exact cap its
    sibling resolveLinkedIssueHardRule already enforces via
    extractLinkedIssueNumbersWithOverflow, rather than adding a second bound.
    Fails open rather than truncating-and-checking-a-subset, since a partial
    check could otherwise wrongly conclude "no open reference" based on data
    that never looked at every citation.
  • src/settings/agent-actions.ts: the stale-disposition-label cleanup loop
    (added in fix(review): clear stale disposition labels once the underlying hold resolves #3575) now precomputes a lowercase Set of the PR's live labels
    instead of re-scanning the label array per candidate, and dedupes by
    lowercase label string so two settings misconfigured to the identical
    string only produce one remove action instead of two.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused — 4 source files + their test files, no unrelated changes.
  • Follows CONTRIBUTING.md; no site//CNAME/VitePress changes.
  • Linked issue: Closes #3613.

Validation

  • git diff --check
  • npm run actionlint — not run; no workflow files touched.
  • npm run typecheck (clean)
  • npm run test:coverage (full/unsharded) — not run locally; ran the specific
    affected files instead (agent-actions.test.ts 230 tests,
    unlinked-issue-candidates.test.ts 15, unlinked-issue-match.test.ts 18,
    linked-issue-hard-rules.test.ts 53, plus the full queue.test.ts
    integration suite, 574 tests) — all green. Cross-referenced the exact new-diff
    line/branch ranges against a fresh coverage-final.json
    (--coverage.include per touched file): 100% of the new statements and
    branches are covered. GitHub CI runs the full suite/gate on push.
  • npm run test:workers / npm run build:mcp / npm run test:mcp-pack — not run;
    nothing in those surfaces touched.
  • npm run ui:openapi:check / ui:lint / ui:typecheck / ui:build — not run; no
    API/schema or apps/gittensory-ui/** changes.
  • npm audit --audit-level=moderate — not run; no dependency changes.
  • New/changed behavior has unit tests for every new branch: the ASCII marker
    assertion updated in place, the basename-boundary false-positive case AND
    the still-matches-a-longer-path case, the linked-issue fan-out bound (both
    over-the-cap and exactly-at-the-cap), the regex-no-match fallback in
    issueMentionsChangedPath, and the disposition-label dedupe when two
    settings collide on the same string.

If any required check was skipped, explain why:

  • Local validation here was typecheck + the specific affected test files (plus
    the full queue.test.ts integration suite) rather than a full local
    test:ci/test:coverage/npm audit pass, since GitHub CI runs the complete
    gate on push and re-running the whole suite by hand for every change is
    redundant.

Safety

  • No secrets, wallets, hotkeys, coldkeys, trust scores, or private scoring values are
    touched or exposed.
  • No public-facing text changes (the touched strings are an internal AI-prompt
    truncation marker and internal label-action reason strings, not new PR-comment
    content).
  • Not an auth/CORS/session change; no negative-path tests needed for that reason.
  • Not an API/OpenAPI change.
  • Not a UI change (N/A) — no UI Evidence section included.
  • No changelog edit.

@JSONbored
fix(review): address non-blocking review nits across the credibility-…
06cf6d2 
…gate-farming series

- unlinked-issue-match.ts: use an ASCII "..." truncation marker instead of a
  Unicode ellipsis, matching this repo's ASCII-source convention.
- unlinked-issue-candidates.ts: drop a duplicate "would" entry in STOPWORDS;
  switch basename path-mention matching from a raw substring .includes() to
  an exact path-token match (or a longer path ending in /basename), so a
  basename like "reader.ts" no longer false-positives inside an unrelated,
  longer filename such as "csv-reader.ts".
- linked-issue-hard-rules.ts: bound resolveLinkedIssueHasOpenReference's
  per-issue fetch fan-out to the same MAX_LINKED_ISSUE_NUMBERS cap its
  sibling resolveLinkedIssueHardRule already enforces via its own overflow
  check, failing open rather than firing an unbounded burst of live fetches
  for a body citing more references than can be safely verified in one pass.
- agent-actions.ts: the stale-disposition-label cleanup loop now precomputes
  a lowercase Set of the PR's live labels instead of re-scanning the array
  per candidate, and dedupes defensively so two label settings misconfigured
  to the identical string only produce one remove action.
@superagent-security

superagent-security Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-05 19:43:49 UTC

8 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a low-risk nits/cleanup PR closing #3613, addressing six previously-flagged non-blocking items from #3513/#3575: an ASCII truncation marker, a duplicate STOPWORDS entry, a tightened basename-vs-substring path-match check, a bounded fail-open cap on resolveLinkedIssueHasOpenReference's fan-out (reusing the existing MAX_LINKED_ISSUE_NUMBERS constant), and a dedupe of the stale-disposition-label cleanup loop. Each change is accompanied by a targeted test exercising the new branch (boundary substring case, exactly-at-cap fanout, dedupe of identically-configured labels, and the empty-path-token fallback), and CI is green across build/typecheck/coverage/security scans. The basename-token regex introduces a minor precision/recall tradeoff (trailing punctuation after a mentioned filename, e.g. "reader.ts.", no longer matches) but this is a soft heuristic signal, not a correctness-critical path, so it doesn't rise to a blocker.

Nits — 5 non-blocking
  • src/signals/unlinked-issue-candidates.ts issueMentionsChangedPath: the new path-token regex `[a-z0-9_\-./]+` will swallow trailing punctuation into the token (e.g. a body mentioning "reader.ts." or "reader.ts," produces token "reader.ts."), so an exact-match basename mention followed by sentence punctuation now silently fails to match — worth a quick check whether that's an acceptable precision/recall tradeoff or needs a trim of trailing punctuation.
  • src/settings/agent-actions.ts: confirm `hasLabel` (now replaced by direct Set lookups) didn't do any normalization beyond lowercasing (e.g. trimming) that the new `livePrLabels`/`alreadyHandled` sets don't replicate, since the diff assumes lowercase-only equivalence.
  • Consider adding a test case for a basename mention immediately followed by punctuation (period/comma) to document the new tokenizer's behavior at that boundary, since it's a subtle change in match semantics from pure substring containment.
  • The `alreadyHandled` dedupe key and `livePrLabels` Set are both keyed on lowercase strings — a brief comment noting the assumption that label casing is otherwise preserved for the actual remove action (only the lookup key is lowercased) would help a future reader confirm no casing bug was introduced.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3613
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 493 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 493 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 56 PR(s), 493 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.19%. Comparing base (8662e8f) to head (06cf6d2).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #3615   +/-   ##
=======================================
  Coverage   93.19%   93.19%           
=======================================
  Files         313      313           
  Lines       31874    31882    +8     
  Branches    11666    11669    +3     
=======================================
+ Hits        29705    29713    +8     
  Misses       1517     1517           
  Partials      652      652
Files with missing lines Coverage Δ
src/review/linked-issue-hard-rules.ts 98.11% <100.00%> (+0.03%) ⬆️
src/review/unlinked-issue-match.ts 100.00% <100.00%> (ø)
src/settings/agent-actions.ts 97.59% <100.00%> (+0.05%) ⬆️
src/signals/unlinked-issue-candidates.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

loopover-orb[bot]
loopover-orb Bot approved these changes Jul 5, 2026
View reviewed changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit f17d68a into main Jul 5, 2026
10 checks passed
@loopover-orb
loopover-orb Bot deleted the claude/review-nits-cleanup branch July 5, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loopover-orb loopover-orb[bot] loopover-orb[bot] approved these changes

Assignees

@JSONbored JSONbored

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

chore(review): address non-blocking review nits from the credibility-gate-farming series

1 participant

@JSONbored